From 5a04a4d48ccb40fa7c81afb53098170019f32095 Mon Sep 17 00:00:00 2001 From: Mark Buer Date: Fri, 24 Jul 2015 18:35:53 +1200 Subject: [PATCH] Enables support for relative pathed linker and ar tools. --- src/cargo/ops/cargo_compile.rs | 7 +- src/cargo/ops/cargo_rustc/context.rs | 10 +-- src/cargo/ops/cargo_rustc/mod.rs | 18 ++-- src/cargo/util/config.rs | 28 ++++--- src/doc/config.md | 21 +++-- tests/test_cargo_tool_paths.rs | 120 +++++++++++++++++++++++++++ tests/tests.rs | 1 + 7 files changed, 170 insertions(+), 35 deletions(-) create mode 100644 tests/test_cargo_tool_paths.rs diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index a92720804..01b5ceb76 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -409,12 +409,9 @@ fn scrape_target_config(config: &Config, triple: &str) -> CargoResult { let key = format!("target.{}", triple); - let ar = try!(config.get_string(&format!("{}.ar", key))); - let linker = try!(config.get_string(&format!("{}.linker", key))); - let mut ret = ops::TargetConfig { - ar: ar.map(|p| p.0), - linker: linker.map(|p| p.0), + ar: try!(config.get_path(&format!("{}.ar", key))), + linker: try!(config.get_path(&format!("{}.linker", key))), overrides: HashMap::new(), }; let table = match try!(config.get_table(&key)) { diff --git a/src/cargo/ops/cargo_rustc/context.rs b/src/cargo/ops/cargo_rustc/context.rs index 2782213dd..5d0e583d4 100644 --- a/src/cargo/ops/cargo_rustc/context.rs +++ b/src/cargo/ops/cargo_rustc/context.rs @@ -1,8 +1,8 @@ use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::collections::{HashSet, HashMap}; +use std::path::{Path, PathBuf}; use std::str; use std::sync::Arc; -use std::path::PathBuf; use regex::Regex; @@ -451,13 +451,13 @@ impl<'a, 'cfg> Context<'a, 'cfg> { } /// Get the user-specified linker for a particular host or target - pub fn linker(&self, kind: Kind) -> Option<&str> { - self.target_config(kind).linker.as_ref().map(|s| &s[..]) + pub fn linker(&self, kind: Kind) -> Option<&Path> { + self.target_config(kind).linker.as_ref().map(|s| s.as_ref()) } /// Get the user-specified `ar` program for a particular host or target - pub fn ar(&self, kind: Kind) -> Option<&str> { - self.target_config(kind).ar.as_ref().map(|s| &s[..]) + pub fn ar(&self, kind: Kind) -> Option<&Path> { + self.target_config(kind).ar.as_ref().map(|s| s.as_ref()) } /// Get the target configuration for a particular host or target diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 486ce8d5c..78cffdc04 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -1,6 +1,6 @@ use std::collections::{HashSet, HashMap}; use std::env; -use std::ffi::OsString; +use std::ffi::{OsStr, OsString}; use std::fs; use std::io::prelude::*; use std::path::{self, Path, PathBuf}; @@ -47,8 +47,8 @@ pub struct BuildConfig { #[derive(Clone, Default)] pub struct TargetConfig { - pub ar: Option, - pub linker: Option, + pub ar: Option, + pub linker: Option, pub overrides: HashMap, } @@ -687,9 +687,11 @@ fn build_base_args(cx: &Context, fn build_plugin_args(cmd: &mut CommandPrototype, cx: &Context, pkg: &Package, target: &Target, kind: Kind) { fn opt(cmd: &mut CommandPrototype, key: &str, prefix: &str, - val: Option<&str>) { + val: Option<&OsStr>) { if let Some(val) = val { - cmd.arg(key).arg(&format!("{}{}", prefix, val)); + let mut joined = OsString::from(prefix); + joined.push(val); + cmd.arg(key).arg(joined); } } @@ -697,11 +699,11 @@ fn build_plugin_args(cmd: &mut CommandPrototype, cx: &Context, pkg: &Package, cmd.arg("--emit=dep-info,link"); if kind == Kind::Target { - opt(cmd, "--target", "", cx.requested_target()); + opt(cmd, "--target", "", cx.requested_target().map(|s| s.as_ref())); } - opt(cmd, "-C", "ar=", cx.ar(kind)); - opt(cmd, "-C", "linker=", cx.linker(kind)); + opt(cmd, "-C", "ar=", cx.ar(kind).map(|s| s.as_ref())); + opt(cmd, "-C", "linker=", cx.linker(kind).map(|s| s.as_ref())); } fn build_deps_args(cmd: &mut CommandPrototype, diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 90c971e83..4591db4f7 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -154,6 +154,22 @@ impl Config { } } + pub fn get_path(&self, key: &str) -> CargoResult> { + if let Some((specified_path, path_to_config)) = try!(self.get_string(&key)) { + if specified_path.contains("/") || (cfg!(windows) && specified_path.contains("\\")) { + // An absolute or a relative path + let prefix_path = path_to_config.parent().unwrap().parent().unwrap(); + // Joining an absolute path to any path results in the given absolute path + Ok(Some(prefix_path.join(specified_path))) + } else { + // A pathless name + Ok(Some(PathBuf::from(specified_path))) + } + } else { + Ok(None) + } + } + pub fn get_list(&self, key: &str) -> CargoResult, PathBuf)>> { match try!(self.get(key)) { Some(CV::List(i, path)) => Ok(Some((i, path))), @@ -240,16 +256,8 @@ impl Config { fn get_tool(&self, tool: &str) -> CargoResult { let var = format!("build.{}", tool); - if let Some((tool, path)) = try!(self.get_string(&var)) { - // If this tool has a path separator in it, calculate the path - // relative to the config file (specified by `path`). To do that we - // chop off the `.cargo/config` at the end of the path and then join - // on the tool. - if tool.contains("/") || (cfg!(windows) && tool.contains("\\")) { - let path = path.parent().unwrap().parent().unwrap(); - return Ok(path.join(tool)) - } - return Ok(PathBuf::from(tool)) + if let Some(tool_path) = try!(self.get_path(&var)) { + return Ok(tool_path); } let var = tool.chars().flat_map(|c| c.to_uppercase()).collect::(); diff --git a/src/doc/config.md b/src/doc/config.md index e0678e4a2..d189bb6c5 100644 --- a/src/doc/config.md +++ b/src/doc/config.md @@ -34,6 +34,11 @@ together. All of the following keys are optional, and their defaults are listed as their value unless otherwise noted. +Key values that specify a tool may be given as an absolute path, a relative path +or as a pathless tool name. Absolute paths and pathless tool names are used as +given. Relative paths are resolved relative to the parent directory of the +`.cargo` directory of the config file that the value resides within. + ```toml # An array of paths to local repositories which are to be used as overrides for # dependencies. For more information see the Cargo Guide. @@ -54,15 +59,15 @@ vcs = "none" # literal string "$triple", and it will apply whenever that target triple is # being compiled to. [target] -# For cargo builds which do not mention --target, these are the ar/linker which -# are passed to rustc to use (via `-C ar=` and `-C linker=`). By default these -# flags are not passed to the compiler. +# For cargo builds which do not mention --target, these are the ar/linker tools +# which are passed to rustc to use (via `-C ar=` and `-C linker=`). By default +# these flags are not passed to the compiler. ar = ".." linker = ".." [target.$triple] -# Similar to the above ar/linker configuration, but this only applies to when -# the `$triple` is being compiled for. +# Similar to the above ar/linker tool configuration, but this only applies to +# when the `$triple` is being compiled for. ar = ".." linker = ".." @@ -77,8 +82,8 @@ timeout = 60000 # Timeout for each HTTP request, in milliseconds [build] jobs = 1 # number of jobs to run by default (default to # cpus) -rustc = "rustc" # path to the compiler to execute -rustdoc = "rustdoc" # path to the doc generator to execute +rustc = "rustc" # the rust compiler tool +rustdoc = "rustdoc" # the doc generator tool target-dir = "target" # path of where to place all generated artifacts ``` @@ -95,3 +100,5 @@ Cargo recognizes a few global environment variables to configure how it runs: `rustdoc` instance instead. * `CARGO_TARGET_DIR` - Location of where to place all generated artifacts, relative to the current working directory. + +Settings specified via config files take precedence over those specified via environment variables. diff --git a/tests/test_cargo_tool_paths.rs b/tests/test_cargo_tool_paths.rs new file mode 100644 index 000000000..4c5d3c0ee --- /dev/null +++ b/tests/test_cargo_tool_paths.rs @@ -0,0 +1,120 @@ +use support::{path2url, project, execs}; +use support::{COMPILING, RUNNING}; +use hamcrest::assert_that; + +fn setup() { +} + +test!(pathless_tools { + let target = ::rustc_host(); + + let foo = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [lib] + name = "foo" + "#) + .file("src/lib.rs", "") + .file(".cargo/config", &format!(r#" + [target.{}] + ar = "nonexistent-ar" + linker = "nonexistent-linker" + "#, target)); + + assert_that(foo.cargo_process("build").arg("--verbose"), + execs().with_stdout(&format!("\ +{compiling} foo v0.0.1 ({url}) +{running} `rustc [..] -C ar=nonexistent-ar -C linker=nonexistent-linker [..]` +", compiling = COMPILING, running = RUNNING, url = foo.url()))) +}); + +test!(absolute_tools { + let target = ::rustc_host(); + + // Escaped as they appear within a TOML config file + let config = if cfg!(windows) { + (r#"C:\\bogus\\nonexistent-ar"#, r#"C:\\bogus\\nonexistent-linker"#) + } else { + (r#"/bogus/nonexistent-ar"#, r#"/bogus/nonexistent-linker"#) + }; + + let foo = project("foo") + .file("Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [lib] + name = "foo" + "#) + .file("src/lib.rs", "") + .file(".cargo/config", &format!(r#" + [target.{target}] + ar = "{ar}" + linker = "{linker}" + "#, target = target, ar = config.0, linker = config.1)); + + let output = if cfg!(windows) { + (r#"C:\bogus\nonexistent-ar"#, r#"C:\bogus\nonexistent-linker"#) + } else { + (r#"/bogus/nonexistent-ar"#, r#"/bogus/nonexistent-linker"#) + }; + + assert_that(foo.cargo_process("build").arg("--verbose"), + execs().with_stdout(&format!("\ +{compiling} foo v0.0.1 ({url}) +{running} `rustc [..] -C ar={ar} -C linker={linker} [..]` +", compiling = COMPILING, running = RUNNING, url = foo.url(), ar = output.0, linker = output.1))) +}); + +test!(relative_tools { + let target = ::rustc_host(); + + // Escaped as they appear within a TOML config file + let config = if cfg!(windows) { + (r#".\\nonexistent-ar"#, r#".\\tools\\nonexistent-linker"#) + } else { + (r#"./nonexistent-ar"#, r#"./tools/nonexistent-linker"#) + }; + + // Funky directory structure to test that relative tool paths are made absolute + // by reference to the `.cargo/..` directory and not to (for example) the CWD. + let origin = project("origin") + .file("foo/Cargo.toml", r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [lib] + name = "foo" + "#) + .file("foo/src/lib.rs", "") + .file(".cargo/config", &format!(r#" + [target.{target}] + ar = "{ar}" + linker = "{linker}" + "#, target = target, ar = config.0, linker = config.1)); + + let foo_path = origin.root().join("foo"); + let foo_url = path2url(foo_path.clone()); + let prefix = origin.root().into_os_string().into_string().unwrap(); + let output = if cfg!(windows) { + (format!(r#"{}\.\nonexistent-ar"#, prefix), + format!(r#"{}\.\tools\nonexistent-linker"#, prefix)) + } else { + (format!(r#"{}/./nonexistent-ar"#, prefix), + format!(r#"{}/./tools/nonexistent-linker"#, prefix)) + }; + + assert_that(origin.cargo_process("build").cwd(foo_path).arg("--verbose"), + execs().with_stdout(&format!("\ +{compiling} foo v0.0.1 ({url}) +{running} `rustc [..] -C ar={ar} -C linker={linker} [..]` +", compiling = COMPILING, running = RUNNING, url = foo_url, ar = output.0, linker = output.1))) +}); diff --git a/tests/tests.rs b/tests/tests.rs index b82c2ecf5..dc80ddd41 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -54,6 +54,7 @@ mod test_cargo_run; mod test_cargo_rustc; mod test_cargo_search; mod test_cargo_test; +mod test_cargo_tool_paths; mod test_cargo_version; mod test_shell; -- 2.30.2